Skip to content

docs: clarify macOS od command collision - #5219

Merged
AmyShang-alt merged 1 commit into
nexu-io:mainfrom
zycaskevin:fix-macos-od-command-collision-5120
Jul 13, 2026
Merged

docs: clarify macOS od command collision#5219
AmyShang-alt merged 1 commit into
nexu-io:mainfrom
zycaskevin:fix-macos-od-command-collision-5120

Conversation

@zycaskevin

@zycaskevin zycaskevin commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • clarify that macOS, Linux, and WSL2 can resolve bare od to the system octal-dump binary instead of Open Design
  • point macOS desktop app / Homebrew cask users to the Settings → MCP server absolute-path snippet
  • extend the install.sh shadowed-binary test so the error guidance covers macOS and the Settings fallback

Fixes #5120.

Surface area

  • README / user-facing MCP install docs
  • Hosted install.sh error guidance for shadowed od binaries
  • Landing-page install.sh static test coverage
  • Runtime CLI / daemon behavior
  • Desktop app packaging, DMG, or Homebrew cask install behavior
  • User config, persisted data, or migrations

Validation

  • node --test apps/landing-page/tests/install-sh-static.test.ts
  • git diff --check
  • Manual smoke of apps/landing-page/public/install.sh with PATH resolving to /usr/bin/od, confirming the stderr now mentions macOS and the Settings → MCP server absolute-path snippet fallback

Test plan

  • node --test apps/landing-page/tests/install-sh-static.test.ts
  • git diff --check

Notes

I also tried pnpm --filter @open-design/landing-page test, but this fresh clone has no node_modules and is running Node v22.22.3 while the repo declares Node ~24, so the package-level test command fails before executing tests with ERR_MODULE_NOT_FOUND: Cannot find package 'tsx'.

The QA queue for the direct install path makes sense; nothing else needed from me on that piece right now.

@lefarcen

lefarcen commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Hey @zycaskevin, thanks for tightening up the macOS od collision path — the README, hosted install.sh, and test update all line up well with the repro in #5120.

Before pool review picks this up, could you add the missing Surface area checklist and a Validation section to the PR body? The current Summary/Test plan already covers the why and user-facing context, so those are the only two template pieces still missing.

Also, because this touches the install path users run directly, we'll keep it in the QA queue before merge. Nothing needed from you on that piece right now.

@lefarcen
lefarcen requested a review from mrcfps July 6, 2026 12:43
@lefarcen lefarcen added size/S PR changes 20-100 lines risk/medium Medium risk: regular code changes type/docs Documentation changes only needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 6, 2026

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zycaskevin I reviewed the README guidance, hosted install.sh error path, and the static installer test updates for the macOS /usr/bin/od collision. The changed copy stays consistent across the user-facing docs and shell fallback, and the focused validation passes locally: node --test apps/landing-page/tests/install-sh-static.test.ts and git diff --check. Thanks for tightening up this install-path edge case and making the recovery path clearer for desktop users.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@lefarcen

lefarcen commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Hey @zycaskevin@mrcfps has signed off on the current head, so this is now queued for QA validation before merge. No action needed from you at the moment; we'll update here once that pass is done.

@lefarcen
lefarcen requested a review from AmyShang-alt July 6, 2026 13:01

@AmyShang-alt AmyShang-alt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA 验收记录

验收范围:

  • README 中 macOS/WSL2 od 命令冲突说明
  • hosted apps/landing-page/public/install.sh 在 PATH 命中非 Open Design od 时的错误提示
  • install.sh 静态测试覆盖

已验证:

  • node --test apps/landing-page/tests/install-sh-static.test.ts 通过,4/4 pass
  • git diff --check 通过
  • 手工模拟 PATH 前置非 Open Design odinstall.sh claude 按预期退出 1,提示 macOS/Linux/WSL2 shadow 问题和 Settings -> MCP server fallback,且未继续执行安装参数写入

未验证:

  • 未验证真实 Electron Settings 页面,因为本 PR 未改 Settings snippet 生成逻辑或桌面打包行为

风险/关注点:

  • 主要风险是文案救援路径是否足够清楚;当前脚本和 README 已覆盖 issue #5120 的 macOS od 冲突场景

结论:

  • 验收通过

@AmyShang-alt AmyShang-alt added validated Runtime change validated (via /explore Pass or manual QA). and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 13, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

Hey @zycaskevin — QA has now signed off on the current head, so that part is all set.

One small housekeeping item is still open in the PR body: the Surface area checklist is still unfilled. Once that’s updated and the remaining CI check settles, this should be in good shape from the review side.

@AmyShang-alt
AmyShang-alt added this pull request to the merge queue Jul 13, 2026
Merged via the queue into nexu-io:main with commit 7332f43 Jul 13, 2026
31 checks passed
@open-design-bot

Copy link
Copy Markdown
Contributor

🎉 📡 You just leveled up to Giotto

Giotto card for @zycaskevin

📡 ✨ Sending steady signals.

🙌 Your contributions are sending a clear signal across the network: you care about making Open Design better. Keep transmitting.

💛 Thanks for helping Open Design move forward. Keep building in the open. 🚀


📊 Rank #158 among 400+ contributors

🔗 Share on X (English) · 分享到 X(中文)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/medium Medium risk: regular code changes size/S PR changes 20-100 lines type/docs Documentation changes only validated Runtime change validated (via /explore Pass or manual QA).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: On macos, the od command conflicts with internal commands, making it unusable

4 participants